refactor: rename persistence -> platform_store + v1 cleanup (#599 PR3/3)#604
Conversation
…3/3) Final PR of issue #599. Renames the slim control-plane store to its real role, refreshes CLAUDE.md, and removes the last v1 stragglers. - git mv codeframe/persistence -> codeframe/platform_store (slim control-plane store: auth, api keys, audit logs, interactive sessions, token usage). - Updated all 31 codeframe.persistence imports across codeframe/, tests/, and scripts/. Also updated active docs: CONTRIBUTING.md, PRD.md, TESTING.md, and the inline python check in .github/workflows/test.yml. - CLAUDE.md: dropped the stale `codeframe/server/` references (the dir does not exist after the v1 cleanup): removed the "Legacy can be read" rule and the matching v2-architecture bullet; replaced the `server/` line in the repo structure tree with `platform_store/`. Refreshed the tests/agents/ note now that only dependency_resolver remains. - Test stragglers (close-out cleanup): - Removed orphan `sample_project_config` fixture from tests/conftest.py (claude-bot note from PR1; zero surviving consumers). - Deleted the 5 remaining #597-era v1 tests that were collect-ignored and referenced removed code: tests/api/{test_project_creation_api, test_workspace_cleanup}, tests/auth/{test_api_key_endpoints, test_authorization_integration, test_dual_auth}. - Un-ignored tests/api/test_health_endpoint.py and fixed one stale v1 assertion (`"database" in data`) — the field was removed from the v2 /health response; the other five assertions match the live contract and restore explicit /health test coverage. - Emptied the root tests/conftest.py collect_ignore. - Deleted 3 v1 ui router tests still listed in tests/ui/conftest.py collect_ignore (test_deployment_mode, test_project_api, test_session_router); left the 2 websocket entries (pre-existing, reference live kept managers). - Deleted tracked stale coverage artifacts (codeframe/{ui/server,platform_store/ database}.py,cover). Historical archives (claudedocs/, legacydocs/, specs/, sprints/, testsprite_tests/) are intentionally untouched — they are frozen point-in-time snapshots, like docs/archive/. Verified: pytest --collect-only clean (3460); /health 200; cf --help; ruff + mypy clean (181 files); tests/api/test_health_endpoint.py 3/3 pass. Closes #599.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
💤 Files with no reviewable changes (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThis PR migrates all imports and re-exports from codeframe.persistence.* to codeframe.platform_store.*, updates application modules to use the platform_store Database/repositories, and synchronizes tests, CI workflows, documentation, and audit tooling to the new paths. ChangesPersistence → Platform Store Reorganization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Address codex finding [P2]: the embedded Python invocation inside the E2E TypeScript global-setup still imported from codeframe.persistence. My earlier sed only touched .py and .md; .ts files with Python string literals were missed. Without this fix the E2E backend setup would fail with ModuleNotFoundError before any test runs. Verified: repo-wide grep across non-historical tracked files now shows zero codeframe.persistence references.
Final Triage Summary (PR #604)Cutoffs: original push 2026-05-29T20:44:53Z; codex-fix push 2026-05-29T20:46:18Z (commit da1dad1). Review passes
Fixed
OutcomeCodex's verdict was the patch is otherwise consistent; this was the only concrete regression. After fix: TRULY CLEAN — every active file uses |
Code Review — PR #604 (refactor: persistence → platform_store, PR3/3)OverviewClean, well-scoped final PR in the #599 cleanup series. The mechanical rename from No blocking issues. Two minor items worth a quick look before merge: Minor Issues1. Stale module docstring in The rename left the original docstring unchanged: """Persistence layer for CodeFRAME state management."""This now conflicts with the stated purpose of the module in the PR description ("slim control-plane store: auth, api keys, audit logs, interactive sessions, token usage"). A one-line update to something like: """Control-plane store: auth, API keys, audit logs, interactive sessions, token usage."""…would keep the docstring consistent with the rename rationale. Low priority, but it's the most visible entry point to the module. 2. The path was updated correctly, but the bullet label was left as PRD.md is historical/legacy context in the repo, so this is genuinely low priority. But if the file is still read by contributors, updating to What Looks Good
SummaryThe rename is clean and complete. The two items above are cosmetic and won't affect correctness or CI. Ready to merge once CI is green on da1dad1. |
Code Review - PR 604: persistence -> platform_store rename + v1 cleanup (3/3)OverviewThis is a clean, mechanical rename PR that closes out the v1 cleanup series. The rename is comprehensive across 31 import sites, CLAUDE.md accurately reflects the new directory layout, and the test cleanup is appropriate. The acceptance criteria look satisfied from the diff. What is Done Well
Minor IssuesIssue 1: Stale docstring in The module-level docstring still reads: The rename captured that this is a control-plane store, not a generic persistence layer. Suggest updating to: Issue 2: Dead import guard in After deleting Issue 3: Section label mismatch in The PRD.md path was updated correctly, but the label was not: the bullet still reads Issue 4: Stale comment block in After emptying No Blocking IssuesNothing here blocks merge. Issues 1 and 2 are the most actionable (quick single-file edits); issues 3 and 4 are cosmetic. The rename is correct and complete. Reviewed by claude[bot] |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/blockers/test_blocker_expiration_simple.py`:
- Line 7: The test is not marked for the v2-only CI and the swapped
platform_store.Database lacks the blocker API expected by tests; add
pytest.mark.v2 (or pytestmark = pytest.mark.v2) to
tests/blockers/test_blocker_expiration_simple.py so it runs under -m v2, and
restore the expected API on codeframe.platform_store.database by adding the
missing methods expire_stale_blockers and get_blocker (implement them as thin
wrappers or adapters that call the new underlying implementation or delegate to
the new Database internals) so the test can exercise the same identifiers it
expects while keeping the new Database structure.
In `@tests/lib/test_metrics_tracker.py`:
- Line 10: The test inserts into legacy v1 tables (projects/tasks) but only
calls Database(":memory:").initialize(), which only builds the slim
control-plane schema via SchemaManager.create_schema(), causing SQL errors; fix
by either (A) ensuring the legacy schema is created before inserts—e.g., after
Database(":memory:").initialize() invoke the schema builder that creates v1
tables (call whatever routine creates legacy tables such as
SchemaManager.create_legacy_schema() or a Database method that provisions legacy
projects/tasks) or explicitly create the projects and tasks tables in the test
setup, and (B) mark the module to run under v2 by adding pytestmark =
pytest.mark.v2 at the top of tests/lib/test_metrics_tracker.py so pytest -m v2
includes this file. Ensure changes reference Database.initialize and the
SchemaManager/legacy schema creation routine (or explicit CREATE TABLE
statements) and add the pytestmark declaration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4a9db77a-dde7-4a13-b910-32322bf0e61b
📒 Files selected for processing (44)
.github/workflows/test.ymlCLAUDE.mdCONTRIBUTING.mdPRD.mdTESTING.mdcodeframe/auth/api_key_router.pycodeframe/auth/dependencies.pycodeframe/cli/auth_commands.pycodeframe/cli/stats_commands.pycodeframe/core/api_key_service.pycodeframe/core/react_agent.pycodeframe/lib/audit_logger.pycodeframe/lib/metrics_tracker.pycodeframe/platform_store/__init__.pycodeframe/platform_store/database.pycodeframe/platform_store/database.py,covercodeframe/platform_store/repositories/__init__.pycodeframe/platform_store/repositories/api_key_repository.pycodeframe/platform_store/repositories/audit_repository.pycodeframe/platform_store/repositories/base.pycodeframe/platform_store/repositories/interactive_sessions.pycodeframe/platform_store/repositories/token_repository.pycodeframe/platform_store/schema_manager.pycodeframe/ui/routers/costs_v2.pycodeframe/ui/server.pycodeframe/ui/server.py,coverscripts/audit_mocked_tests.pytests/api/test_health_endpoint.pytests/auth/test_api_key_endpoints.pytests/auth/test_api_key_repository.pytests/auth/test_authorization_integration.pytests/auth/test_dual_auth.pytests/blockers/test_blocker_expiration_simple.pytests/cli/test_api_key_commands.pytests/conftest.pytests/e2e/global-setup.tstests/integration/conftest.pytests/lib/test_metrics_tracker.pytests/test_issues.mdtests/ui/conftest.pytests/ui/test_deployment_mode.pytests/ui/test_project_api.pytests/ui/test_session_router.pytests/unit/test_interactive_sessions_api.py
💤 Files with no reviewable changes (8)
- tests/auth/test_authorization_integration.py
- tests/ui/test_project_api.py
- tests/api/test_health_endpoint.py
- tests/ui/test_deployment_mode.py
- tests/auth/test_dual_auth.py
- tests/auth/test_api_key_endpoints.py
- tests/ui/test_session_router.py
- codeframe/ui/server.py,cover
CodeRabbit caught two pre-existing PR2 leftovers I missed (Major findings, verified real — both error at fixture setup against the slimmed schema); claude-bot raised four minor docstring/comment items. CodeRabbit fixes (deletions): - tests/blockers/test_blocker_expiration_simple.py: calls removed Database.expire_stale_blockers / get_blocker and inserts into the dropped projects table. 7 errors when run; deselected by -m v2 so CI stayed green. - tests/lib/test_metrics_tracker.py: inserts into dropped projects/tasks tables in fixtures. 29 tests, all deselected by -m v2; MetricsTracker live exercise is covered by tests/core/test_react_agent_tokens.py. claude-bot fixes (clarity): - codeframe/platform_store/__init__.py: update docstring from "Persistence layer for CodeFRAME state management" to reflect the actual control-plane role. - tests/ui/conftest.py: drop the dead "Guard v1 import" try/except for Database (the import always succeeds now); replace it with a direct import and tighten the comment above collect_ignore to explain what those two remaining entries actually are. - tests/conftest.py: replace the stale comment block above the now-empty collect_ignore with a one-line note. - PRD.md: update the bullet label from "Persistence" to "Platform store" to match the rename. Skipped (claude-bot finding #2 was incorrect): "remove dead Database import guard" is partly wrong — Database IS used at tests/ui/conftest.py:88 and :137 (running_server fixture). Kept the import (simplified to a direct one) instead of removing it. Verified: ruff + mypy clean (181); pytest --collect-only clean (3424); focused v2 sample (auth + token + interactive + health) 25/25 pass.
Final Triage Summary (PR #604) — round 2Cutoffs: original push 2026-05-29T20:44:53Z; codex-fix push 2026-05-29T20:46:18Z; review-fix push 2026-05-29T21:00:04Z (commit faa8c96). Review passes
Fixed in faa8c96
Skipped (with one-line justification)
OutcomeNo unresolved Critical/Major findings. After fixes: ruff + mypy clean (181 files); collect-only clean (3424); focused v2 sample (auth + token + interactive + health) 25/25 pass. Ready to merge once CI is green on faa8c96. |
Follow-up Review — PR #604 (da1dad1)Picking up from my prior review and frankbria's triage summary. Both open items are closed. Items Resolved1. Module docstring (flagged in prior review) """Platform store: control-plane state (auth, API keys, audit logs, interactive sessions, token usage)."""This was accurate as shipped — no follow-up needed. 2. TypeScript embedded Python string (codex P2, fixed in da1dad1) Current State: Clean
One Observation (Non-blocking)
Verdict: ready to merge once CI is green on da1dad1. The rename is complete and consistent. |
1 participant
Summary
Final PR (3 of 3) for #599 — closes the v1 cleanup series. Renames the slim control-plane store to its real role, refreshes CLAUDE.md, and removes the last v1 stragglers.
Net: 43 files changed, +49 / −2,411.
git mv codeframe/persistence → codeframe/platform_store(the slim control-plane store: auth, api keys, audit logs, interactive sessions, token usage).codeframe.persistenceimports acrosscodeframe/,tests/, andscripts/. Also updated active docs:CONTRIBUTING.md,PRD.md,TESTING.md, and the inline python check in.github/workflows/test.yml.codeframe/server/references (the dir does not exist after the v1 cleanup) — removed the "Legacy can be read" rule + the matching v2-architecture bullet; replaced theserver/line in the repo-structure tree withplatform_store/; refreshed thetests/agents/note now that onlydependency_resolverremains.sample_project_configfixture fromtests/conftest.py(claude-bot note from PR1; zero surviving consumers).tests/api/{test_project_creation_api,test_workspace_cleanup},tests/auth/{test_api_key_endpoints,test_authorization_integration,test_dual_auth}.tests/api/test_health_endpoint.pyand fixed one stale v1 assertion ("database" in data) — the field was removed from the v2/healthresponse; the other five assertions match the live contract and restore explicit/healthtest coverage.tests/conftest.pycollect_ignore.tests/ui/conftest.pycollect_ignore(test_deployment_mode,test_project_api,test_session_router); left the 2 websocket entries (pre-existing, reference live kept managers).Historical archives (
claudedocs/,legacydocs/,specs/,sprints/,testsprite_tests/) are intentionally untouched — they are frozen point-in-time snapshots, likedocs/archive/.Acceptance Criteria (PR3 + remaining #599 criteria)
codeframe/persistence/renamed tocodeframe/platform_store/; all imports updatedserver/referencecodeframe serveboots (TestClient lifespan/health→ 200)pytest --collect-onlyclean;uv run pytest -m v2greencf --help+ golden-path work; ruff + mypy cleanWith PR1 (#602) + PR2 (#603) merged, this PR completes every issue #599 criterion: no legacy multi-agent classes;
core/project.py+ dead dirs removed; plan engine documented as kept;Databaseexposes only control-plane methods; rename done; CLAUDE.md refreshed.Test Plan
pytest --collect-onlyclean (3460 tests)uv run pytest -m v2green (2824 passed, 636 deselected, 0 failed)/health→ 200cf --help+cf statusworkruff check codeframe/clean;mypy codeframe/clean (181 files)tests/api/test_health_endpoint.py3/3 pass (live/healthcoverage restored)codeframe.persistence/codeframe/persistence/Known Limitations / Intentionally Deferred
claudedocs/,legacydocs/,specs/,sprints/) — frozen by convention.token_repository/audit_repositorythread-safety,save_token_usageINSERT duplication, async PRAGMA repeated 3×) remain deferred to a separate "harden surviving repos" PR.tests/test_new_feature.pyunused-import (ruff F401) — out of scope; not introduced by this PR.Closes #599.
Summary by CodeRabbit
Chores
Documentation